Skip to content
This repository has been archived by the owner on Feb 9, 2022. It is now read-only.

[mdanalysis] include new text selectors #4700

Merged
merged 2 commits into from
Oct 6, 2021
Merged

Conversation

orbeckst
Copy link
Contributor

@orbeckst orbeckst commented Oct 6, 2021

Pull request motivation(s)

What is the current behaviour?

If the current behaviour is a bug, please provide all the steps to reproduce and screenshots with context.

What is the expected behaviour?

NB: Do you want to request a feature or report a bug?
NB2: Any other feedback / questions ?

* new text selectors for definitions (dt tags) in dl (definition lists) as used in the sphinx-generated API docs (to index the function/class names themselves);
  dd (data tags) do not need a selector because text content is wrapped in p tags inside the dd and is already selected
* new text selectors for pre (code blocks)
* see issue algolia#4699
@hmacdope
Copy link

hmacdope commented Oct 6, 2021

Just so you know I'm no expert on this stuff @orbeckst. If you are happy with it then merge away. :)

@orbeckst
Copy link
Contributor Author

orbeckst commented Oct 6, 2021

I can’t merge this PR by myself.

I need an algolia dev such as @s-pace to have a look.

@hmacdope
Copy link

hmacdope commented Oct 6, 2021

I am stupid, i thought this was on a different repo.

@@ -23,10 +23,10 @@
"lvl2": "[itemprop='articleBody'] > .section h3, .page h3, .post h3, .body > .section h3",
"lvl3": "[itemprop='articleBody'] > .section h4, .page h4, .post h4, .body > .section h4",
"lvl4": "[itemprop='articleBody'] > .section h5, .page h5, .post h5, .body > .section h5",
"text": "[itemprop='articleBody'] > .section p, .page p, .post p, .body > .section p, [itemprop='articleBody'] > .section li, .page li, .post li, .body > .section li"
"text": "[itemprop='articleBody'] > .section p, .page p, .post p, .body > .section p, [itemprop='articleBody'] > .section li, .page li, .post li, .body > .section li, [itemprop='articleBody'] > .section dt, .body > .section dt, [itemprop='articleBody'] > .section pre, .page pre, .post pre, .body > .section pre, "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, having this number of selectors could add a lot of noise to the search results and hide other relevant results. A new crawl will be started right after the merge, don't hesitate to revert if you're not happy with the results!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the comment @shortcuts .

Most of the selectors were added earlier by @s-pace to properly index our sites. If adding dt and pre to this list creates too much noise we'll take you up on your offer to revert.

Thank you for the opportunity to try it out.

I hope that this also somehow solves #4699 .

@shortcuts shortcuts merged commit 5e75250 into algolia:master Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants